Repository navigation
feat(workflows): single step-type registry for handler + field validation - #575
Merged
Merged
Conversation
…validation Introduces WP_Agent_Workflow_Step_Type_Registry, pairing each workflow step type with its handler and an optional field validation contract (a simple `required` field-name list, and/or a `validate` callback). register_workflow_step_type() is the public helper. Built-in types (ability, agent, foreach, parallel) register lazily on first access; their field checks moved out of WP_Agent_Workflow_Spec_Validator's inline per-type ifs and into the registry's validate callbacks, byte-identical in error codes/paths/ messages. Deletes the three duplicated default handler maps (runner constructor, runner::default_step_handlers(), reconcile branch resolver) in favor of one WP_Agent_Workflow_Step_Type_Registry::handlers() call each. Duplicate register() calls keep the first registration and trigger _doing_it_wrong. Back-compat for one release: the legacy wp_agent_workflow_known_step_types and wp_agent_workflow_step_handlers filters still apply on top of the registry's output, marked @deprecated. KNOWN_STEP_TYPES stays for BC but is documented as no longer the source of truth. Fixes #574
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #574
API
WP_Agent_Workflow_Step_Type_Registry— one registry pairing a step type name with its runner handler and an optional field-validation contract:register( string $type, array $args ): bool,get(),all(),types(),handlers(),validate_step( array $step, string $path ): array,reset()(test-only).ability,agent,foreach,parallel) register lazily on first access — noinithook dependency, so it behaves identically in plain-PHP smoke tests and inside WordPress.register()for an already-registered type keeps the first registration and triggers_doing_it_wrong(guarded byfunction_exists()), returnsfalse. No silent overwrite.WP_Agent_Workflow_Spec_Validator::known_step_types()(new, public) is the registry'stypes()extended by the legacy filter — replaces the inlineKNOWN_STEP_TYPEScheck.WP_Agent_Workflow_Spec_Validator::validate_steps()bumped fromprivatetopublicso the registry's built-inforeach/parallelvalidate callbacks can recurse into nested step lists through the same function, instead of a second copy living in the registry.Back-compat plan (one release)
wp_agent_workflow_known_step_typesandwp_agent_workflow_step_handlersare still applied on top of the registry's output, in the runner constructor,default_step_handlers(),agents_workflow_resolve_step_handlers(), andWP_Agent_Workflow_Spec_Validator::known_step_types(). All four call sites carry@deprecated 0.14.0docblocks pointing atregister_workflow_step_type().WP_Agent_Workflow_Spec_Validator::KNOWN_STEP_TYPESconst is kept (some consumer may still read it directly) but its docblock now says explicitly it is not the source of truth as of 0.14.0 —WP_Agent_Workflow_Step_Type_Registry::types()is.Deleted duplicate maps
Three inline
array( 'ability' => ..., 'agent' => ..., 'foreach' => ..., 'parallel' => ... )maps deleted, each replaced by oneWP_Agent_Workflow_Step_Type_Registry::handlers()call:WP_Agent_Workflow_Runner::__construct()(was ~line 76)WP_Agent_Workflow_Runner::default_step_handlers()(was ~line 1698)agents_workflow_resolve_step_handlers()inregister-reconcile-workflow-branch.php(was ~line 958)git grep "'parallel' => array( "now returns zero matches (the acceptance criteria's "one, not three" — the built-ins are registered as four separateself::register()calls inregister_builtin_types(), not one combined array literal, so there's no map at all left to find, duplicated or otherwise).Field validation moved into the registry
ability(required => ['ability']) andagent(required => ['agent','message']) use the registry's generic required-field check — their original messages ("{type} step is missing a non-empty \{field}`"`) match the generic format exactly, byte-for-byte.foreachandparallelget dedicatedvalidatecallbacks in the registry (moved verbatim from the validator's old inlineif ('foreach' === $step['type'])/validate_parallel_step()), recursing into nested step lists via the now-publicWP_Agent_Workflow_Spec_Validator::validate_steps().One correctness note found while moving this:
is_callable()on a[ClassString, 'method']pair triggers autoloading to verify the method exists. The registry's own built-ins point atWP_Agent_Workflow_Runner, which several existing workflow smoke tests deliberately don't load (e.g.workflow-spec-validator-smoke.phponly loads the validator + spec). A strictis_callable()gate at registration silently rejected the built-ins in that context. Registration now does a structural shape check (Closure/ non-empty string /[target, method]pair / invokable object) instead, deferring real callability tocall_user_func()at dispatch time — same trust model the pre-registry code had (it stored these as plain array values without ever validating them).Tests
New
tests/workflow-step-type-registry-smoke.php(added tocomposer smoke), 27 assertions:required+validate: missing required field fails with the right path/code; thevalidatecallback's own rule fails with the right path/code; a fully valid step passes; the registered handler actually runs viahandlers()and returns its output.false, triggers_doing_it_wrong, and keeps the first handler (confirmed by invoking it).wp_agent_workflow_known_step_types+wp_agent_workflow_step_handlers) still passes structural validation and runs end-to-end throughWP_Agent_Workflow_Runner.ability/agent/foreach/parallelerror codes, paths, and (forability) the exact message are unchanged after the move, including a foreach-nested-step path check through the shared recursive validator.Existing
workflow-spec-validator-smoke.phpand the 11 other smoke files that load the validator/runner/reconcile-branch file now also loadclass-wp-agent-workflow-step-type-registry.php; no assertions in any of them changed.Results
AI disclosure
AI assistance: Yes / Tool: OpenCode / Used for: implementation and tests